image-layout: clarification of oci-layout by vbatts · Pull Request #434 · opencontainers/image-spec

vbatts · 2016-11-01T16:46:17Z

The section for this file was too crammed together. Breaking it out into
individual items allows for more straightforward validation.

Signed-off-by: Vincent Batts vbatts@hashbangbash.com

wking · 2016-11-01T16:55:07Z

image-layout.md


- "oci-layout" MUST contain a JSON object with a version field `{"imageLayoutVersion": "1.0.0"}` and MAY include additional fields.
+- It MUST be a JSON object
+- It MUST contain a `imageLayoutVersion` field for determing the version of the layout used


“a imageLayoutVersion” → “an imageLayoutVersion” and “determing” → “determining”.

I'm also fine with the shorter:

It MUST contain an imageLayoutVersion field.

since the purpose of that field seems fairly obvious.

wking · 2016-11-01T16:55:47Z

image-layout.md



-It also contains a file that is used to identify the layout version:
+The image layout MUST contain a file "oci-layout":


Instead of quoting, can we use backticks (oci-layout)?

wking · 2016-11-01T17:02:48Z

image-layout.md

- "oci-layout" MUST contain a JSON object with a version field `{"imageLayoutVersion": "1.0.0"}` and MAY include additional fields.
+- It MUST be a JSON object
+- It MUST contain a `imageLayoutVersion` field for determing the version of the layout used
+    - The `imageLayoutVersion` value will align with the `specs.Version` at the time changes to the layout are made, and will pin a given version until changes to the layout are required


It may be worth pointing out that this means imageLayoutVersion is not semantically versioned. I'm ok with that, since I expect most image-layout bumps will break backwards compat anyway, but my personal preference would be to have a semantically versioned imageLayoutVersion that is more independent of the spec version. You'd bump the spec version if any constituent component added a new feature (minor bump) or a backwards-incompatible feature (major bump). So:

A major imageLayoutVersion bump would require a major spec version bump.

A minor imageLayoutVersion bump would require at least a minor spec version bump.

A new layer media type (e.g. for Any chance of changing the whiteout file approach? #24) would require a minor spec version bump but no imageLayoutVersion bump.

Dropping an old media type (e.g. after types based on .wh.* fall out of use) would require a major spec version bump but no imageLayoutVersion bump.

Also, I'd rather change “specs.Version” → “OCI Image Format Specification version” to avoid referencing the informative Go from Markdown spec.

wking · 2016-11-02T05:00:16Z

image-layout.md

- "oci-layout" MUST contain a JSON object with a version field `{"imageLayoutVersion": "1.0.0"}` and MAY include additional fields.
+- It MUST be a JSON object
+- It MUST contain an `imageLayoutVersion` field
+    - The `imageLayoutVersion` value will align with the OCI Image Specification version at the time changes to the layout are made, and will pin a given version until changes to the layout are required


You've gone with “OCI Image Specification”, but the title for this spec is “Open Container Initiative Image Format Specification”. I'm fine changing “Open Container Initiative” → “OCI”, but don't think we should be dropping “Format” here unless we also drop it from the spec title. I don't mind which way we go, but I think we should stay consistent.

I also still think we should either semantically version imageLayoutVersion or point out that this is not semantically versioned.

I went with this because the image-layout.md doc is titled "OCI Image Layout Specification".

This file is about the OCI Image Layout Specification, and if imageLayoutVersion semantically versions that, I'm happy. However, it seems like the “pin” language was adding a relationship to the project-wide OCI Image Format Specification version.

To distinguish the two cases, pretend we had already cut 1.0.0, and say there is a backwards-compatible change to the image layout (e.g. you add an optional index, #438) and a backwards-incompatible change to something else (e.g. we drop support for .wh.*, #24). The backwards-incompatible change means the project as a whole goes to 2.0.0. With the independently-SemVer'ed approach, the backwards-compatible image-layout change moves imageLayoutVersion to 1.1.0. With the pinned-to-the-project-version approach, imageLayoutVersion moves to 2.0.0. Which of those is your intended result?

Dare I say that some kind of specific-feature based compatibility would most closely be like the bittorrent BEP process, where specific features or enhancements are assigned an indicator (i.e. BEP0001). Rather than rev'ing a version that requires compliance of all features, an implementation can specify the specific features they accommodate.
Your pretending may certainly find benefits and drawbacks to both approaches. Neither is perfect. What is your intended result?

I'm fine with tagged features (opencontainers/runtime-spec#15), but the OCI specs seemed to be leaning towards linearized versioning. With linearized versioning, my preferred result is 1.1.0 (because it avoids confusing implementers and layout authors who are interested in image-layout but not with the other spec features). I don't expect much feature fragmentation in image-layout, so shifting to tagged features would be a small gain. However, just because I don't foresee fragmentation doesn't mean we won't have it ;). So I'm also fine switching to tagged features proactively.

Signed-off-by: Aleksa Sarai <asarai@suse.com>

philips · 2016-11-17T04:20:03Z

LGTM

The section for this file was too crammed together. Breaking it out into individual items allows for more straightforward validation. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2016-11-21T18:53:49Z

updated. PTAL.

jonboulle · 2016-11-28T18:40:33Z

lgtm

stevvooe · 2016-12-07T22:26:50Z

LGTM

Do we have any language about how implementations must error if the Major is different, but must be able to process future minors/patch?

wking · 2016-12-07T23:12:47Z

On Wed, Dec 07, 2016 at 02:26:50PM -0800, Stephen Day wrote: Do we have any language about how implementations must error if the Major is different, but must be able to process future minors/patch?

Are we intending to major-bump the OCI Image Format Specification version when there is a backwards incompatible change to image-layout? Or are we reserving major bumps for backwards-incompatible changes in the blob schemas? As I read it (still with 9c333a), the current wording does not imply semantic versioning for image-layout (and I think we should either require semantic versioning for image-layout or point out that we're not requiring it [1,2]). [1]: #434 (comment) [2]: #434 (comment)

vbatts mentioned this pull request Nov 1, 2016

Add ImageLayoutVersion const #414

Merged

vbatts force-pushed the image-layout_oci-layout_clarification branch 2 times, most recently from d3f520e to daac7a3 Compare November 1, 2016 16:54

wking reviewed Nov 1, 2016

View reviewed changes

vbatts force-pushed the image-layout_oci-layout_clarification branch from daac7a3 to 538e518 Compare November 1, 2016 18:24

wking reviewed Nov 2, 2016

View reviewed changes

wking mentioned this pull request Nov 3, 2016

Separate versioning for refs and CAS portions of image-layout? #441

Closed

wking referenced this pull request in opencontainers/umoci Nov 6, 2016

image: cas: implement CAS engine

667271b

Signed-off-by: Aleksa Sarai <asarai@suse.com>

coolljt0725 mentioned this pull request Nov 18, 2016

image-layout.md: clarify the layout file name MUST be oci-layout #461

Closed

vbatts force-pushed the image-layout_oci-layout_clarification branch from 538e518 to 1c4720b Compare November 21, 2016 18:53

image-layout: clarification of oci-layout

9c333aa

The section for this file was too crammed together. Breaking it out into individual items allows for more straightforward validation. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the image-layout_oci-layout_clarification branch from 1c4720b to 9c333aa Compare November 21, 2016 18:53

jonboulle merged commit 656fb2f into opencontainers:master Dec 7, 2016

vbatts deleted the image-layout_oci-layout_clarification branch December 9, 2016 14:29

vbatts mentioned this pull request Jan 27, 2017

Bump version for v1.0.0-rc4 #537

Merged



		It also contains a file that is used to identify the layout version:
		The image layout MUST contain a file "oci-layout":

Conversation

vbatts commented Nov 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philips commented Nov 17, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbatts commented Nov 21, 2016

Uh oh!

jonboulle commented Nov 28, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevvooe commented Dec 7, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wking commented Dec 7, 2016 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

philips commented Nov 17, 2016 •

edited by caniszczyk

Loading

jonboulle commented Nov 28, 2016 •

edited by caniszczyk

Loading

stevvooe commented Dec 7, 2016 •

edited by caniszczyk

Loading